Skip to content

cli: add access-pass plan and apply - #4281

Open
juan-malbeclabs wants to merge 1 commit into
mainfrom
jo/access-pass-plan-apply
Open

cli: add access-pass plan and apply#4281
juan-malbeclabs wants to merge 1 commit into
mainfrom
jo/access-pass-plan-apply

Conversation

@juan-malbeclabs

@juan-malbeclabs juan-malbeclabs commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Related: RFC-28

Summary of Changes

  • New doublezero access-pass plan and doublezero access-pass apply, which reconcile access passes against a YAML document describing the access a fleet should have. plan reports the difference between the document and the ledger and writes nothing; apply sends the same difference after a confirmation. Granting six servers across four groups in both roles goes from 48 invocations to one reviewable file.
  • The document covers both halves of a pass's access: the multicast publish/subscribe allowlists, and the IBRL (unicast) tenant. Each entry is keyed by (client_ip, user_payer) — the pass's PDA seeds.
  • Every field is declarative. A group the document does not name is revoked; an entry with no ibrl has its tenant cleared. This is what makes the file a description of state rather than a list of additions, and it is why the revocations are shown before they happen.
  • A declared subscribe that an EdgeSeat feed already grants is reported as satisfied and never written. Re-granting would spend a transaction, change nothing, and make every run report as changed — so this is what keeps a converged document a genuine no-op. Publisher is never feed-covered, so a publish gap on the same group is still reported.
  • Two situations are refused rather than attempted, and both exit non-zero: an access pass that does not exist (AddMulticastGroup*Allowlist against an empty PDA silently mints a Prepaid pass with 1/1 seats and last_access_epoch: 0, so a typo'd address would produce a junk pass that looks real), and a group leaving both allowlists at once (the detach verbs send the role being kept as desired state, so revoking both entries first strands the roles on the User account with no legal write to remove them).
  • apply --json writes exactly one object to stdout with changed first, so configuration management can drive it with changed_when. It requires --auto-approve or --dry-run, since there is no terminal to answer the prompt on.
  • No new dependency and no program change: serde_yaml is already in the crate, and all writes go through existing instructions.

Two details worth a reviewer's attention:

  • The IBRL write targets the stored pass, not the declared IP. A pass with allow_multiple_ip lives at the PDA seeded with 0.0.0.0 and serves any client IP, and SetAccessPass seeds its PDA from the client_ip argument — sending the declared address would write a different account than the plan described. plan also warns whenever a declared address resolves to a shared pass.
  • The IBRL write reads the pass first. SetAccessPass overwrites accesspass_type, last_access_epoch, allow_multiple_ip and both seat caps from its arguments, so those are read back and re-sent unchanged; the transaction moves the tenant, pins the epoch, and touches nothing else. Declaring ibrl pins the epoch to unlimited, because the tenant and the epoch are one grant and a finite value fails a later connect ibrl at an unpredictable date.

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 3 +1118 / -0 +1118
Tests 3 +1125 / -0 +1125
Docs 3 +400 / -0 +400
Scaffolding 3 +16 / -0 +16
Total 9 +2659 / -0 +2659

Roughly half the diff is tests, and the tests live inline in the same three files as the core logic. Nothing is modified — every line is new.

Key files (click to expand)
  • smartcontract/cli/src/accesspass/plan.rs (+567 core / +515 tests) — the diff engine and its renderer. Builds the multicast grants/revokes and the IBRL change, applies the feed-coverage and dual-revoke rules, batches the group/tenant/feed reads, and renders the plan.
  • smartcontract/cli/src/accesspass/apply.rs (+371 core / +443 tests) — the write path. Prompts, sends each change, reports per item and continues past failures; contains the read-modify-write for SetAccessPass and the JSON contract.
  • smartcontract/cli/src/accesspass/desired.rs (+180 core / +167 tests) — the document schema, strict parsing, and payer resolution (kept separate from parsing so a document can be validated with no keypair and no network).
  • rfcs/rfc28-declarative-access-passes.md (+223) — design, alternatives considered, and six open questions.
  • docs/access-pass-plan-apply.md (+171) — operator documentation: schema, reading a plan, what it refuses, automation.
  • smartcontract/cli/src/cli/{accesspass,command}.rs, accesspass/mod.rs (+16) — subcommand enum, dispatch and module registration.

Testing Verification

  • 35 new unit tests against a mocked client (10 document, 17 plan, 8 apply); 472 pass in the crate.
  • Idempotency: a document matching the ledger plans nothing, and a second apply reports changed: false and sends zero transactions.
  • Feed coverage: an EdgeSeat pass with an empty mgroup_sub_allowlist but a covering feed reports the group as satisfied via feed <code> and issues no transaction, while still reporting the publisher gap on the same group.
  • Shared-pass targeting: an entry declaring 203.0.113.10 that resolves to a pass stored at 0.0.0.0 sends SetAccessPass with client_ip: 0.0.0.0, preserving the EdgeSeat type, allow_multiple_ip, and 3/5 seat caps, with last_access_epoch pinned to u64::MAX.
  • Refusals: a missing access pass and a group leaving both allowlists each plan zero writes and are reported as blocked.
  • Strict parsing: subscibe: is rejected with the entry index, the valid alternatives and the line/column, before any RPC call — confirmed against the built binary, which exits 1 at parse time while a valid document proceeds to the network stage.
  • Lazy reads: a document with no ibrl skips the tenant scan when no pass carries a tenant, but still reads them when one does — a pass with a tenant has to be cleared, so the scan cannot be skipped merely because the document is silent.
  • Not yet exercised against a live ledger; no local devnet was running. An end-to-end run over dev/local-devnet-multicast-setup.md is the remaining verification.

@juan-malbeclabs
juan-malbeclabs requested review from a team and a lite review from Copilot September 5, 2026 13:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The apply path has confirmed issues in dry-run behavior and JSON reporting semantics that can mislead automation and operators.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds declarative reconciliation for access passes in the DoubleZero CLI via new doublezero access-pass plan and doublezero access-pass apply subcommands, driven by a YAML “desired state” document. It also includes operator docs, an RFC, and changelog entry to document the workflow and safety properties.

Changes:

  • Add YAML schema parsing and normalization for declarative access-pass intent (desired.rs).
  • Implement a diff engine plus renderer for planned multicast allowlist and IBRL tenant changes (plan.rs).
  • Implement an apply path that prompts (or auto-approves), sends per-item transactions, and can emit a single JSON object for automation (apply.rs), plus CLI wiring and documentation.
File summaries
File Description
smartcontract/cli/src/cli/command.rs Dispatch new access-pass plan and access-pass apply commands (apply uses stdin).
smartcontract/cli/src/cli/accesspass.rs Register new Clap subcommands for access-pass planning and applying.
smartcontract/cli/src/accesspass/plan.rs Implement plan building, feed coverage logic, blocked conditions, and human/JSON rendering.
smartcontract/cli/src/accesspass/apply.rs Implement apply execution, confirmation, per-change transaction sends, and JSON output contract.
smartcontract/cli/src/accesspass/desired.rs Define and parse the YAML schema, resolve payers, and normalize group lists.
smartcontract/cli/src/accesspass/mod.rs Export new access-pass modules.
rfcs/rfc28-declarative-access-passes.md Add design RFC describing schema, semantics, and limitations.
docs/access-pass-plan-apply.md Add operator documentation for plan/apply usage and automation.
CHANGELOG.md Document the new CLI functionality and its key behaviors.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

out: &mut W,
input: &mut R,
) -> eyre::Result<()> {
client.check_requirements(CHECK_ID_JSON | CHECK_BALANCE)?;
}

if failed > 0 {
eyre::bail!("{failed} of {} allowlist changes failed", results.len());
Comment on lines +353 to +358
let applied = results.iter().filter(|r| r.state == "applied").count()
+ ibrl_results.iter().filter(|r| r.state == "applied").count();
let total = results.len() + ibrl_results.len();
let json = serde_json::to_string_pretty(&ApplyJson {
changed: applied > 0,
counts: Counts {
Comment on lines +128 to +137
let user_payer = if raw_payer.eq_ignore_ascii_case("me") {
payer
} else {
Pubkey::from_str(raw_payer).map_err(|_| {
eyre::eyre!(
"access_passes[{index}] ({}) has an invalid user_payer: {raw_payer}",
entry.client_ip
)
})?
};

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 8b9f504. Configure here.

}

Ok(())
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plan exits zero when blocked

Medium Severity

Issue
access-pass plan always returns success when the plan contains blocked items.

Context
An operator or CI job runs plan to gate apply. The document names a missing pass or a dual-revoke group. The command prints the blocked items and then exits 0.

Proposed Fix
Have plan return the same blocked-item error that apply already returns.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by project rule: Comment shape

Reviewed by Cursor Bugbot for commit 8b9f504. Configure here.


if failed > 0 {
eyre::bail!("{failed} of {} allowlist changes failed", results.len());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply failure count omits IBRL

Low Severity

Issue
The non-zero exit message counts only multicast results in the denominator, so an IBRL-only failure reports a total of zero.

Context
An operator runs apply and the tenant write fails while every allowlist write succeeds, or only IBRL was planned. The summary line already includes IBRL, but the bail text does not.

Proposed Fix
Use the combined multicast plus IBRL count in the failure message.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8b9f504. Configure here.

Reconcile access passes against a YAML document describing the access a
fleet should have. `plan` reports the difference between the document and
the ledger and writes nothing; `apply` sends the same difference after a
confirmation.

The document covers both halves of a pass's access -- the multicast
publish/subscribe allowlists and the IBRL (unicast) tenant -- keyed by
(client_ip, user_payer), the pass's PDA seeds. Every field is
declarative: a group the document does not name is revoked, and an entry
with no ibrl has its tenant cleared.

A declared subscribe that an EdgeSeat feed already grants is reported as
satisfied and never written. Re-granting would spend a transaction,
change nothing and make every run report as changed, so this is what
keeps a converged document a no-op. Publisher is never feed-covered, so
a publish gap on the same group is still reported.

Two situations are refused rather than attempted, and both exit
non-zero: an access pass that does not exist, since granting against an
empty PDA silently mints a Prepaid pass with 1/1 seats and no epoch; and
a group leaving both allowlists at once, since the detach verbs send the
role being kept as desired state and revoking both entries first strands
the roles on the User account.

The IBRL write reads the pass first and re-sends the type, seat caps and
allow_multiple_ip unchanged, because SetAccessPass overwrites those from
its arguments, and it targets the stored pass so a grant on a shared
0.0.0.0 pass is not written to a different account. Declaring ibrl pins
last_access_epoch to unlimited: the tenant and the epoch are one grant,
and a finite value fails a later connect ibrl at an unpredictable date.

apply --json writes one object with changed first, for configuration
management, and requires --auto-approve since there is no terminal to
confirm on.

Design and open questions in RFC-28; operator documentation in
docs/access-pass-plan-apply.md.
@juan-malbeclabs
juan-malbeclabs force-pushed the jo/access-pass-plan-apply branch from 7e428f5 to aab508d Compare September 5, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants